Skip to content

Fix misleading/confusing error types throughout uxarray - #1652

Open
Sevans711 wants to merge 4 commits into
mainfrom
sevans/error-types-cleanup
Open

Fix misleading/confusing error types throughout uxarray#1652
Sevans711 wants to merge 4 commits into
mainfrom
sevans/error-types-cleanup

Conversation

@Sevans711

Copy link
Copy Markdown
Collaborator

Closes #1622

Overview

Fixes error types which did correspond closely to the actual error being reported. For example, for an error like "This functionality is not yet supported" the error type should be NotImplementedError, not ValueError. This PR addresses all cases mentioned in #1622, which is an exhaustive list of all such cases throughout uxarray.

Test suite has been updated accordingly for all tests which failed due to expecting old error types.

Slight expansion beyond original scope: this PR also adds an import of uxarray.errors module in uxarray's top-level __init__.py, to make custom error types easier to refer to. Now users can reliably write something like ux.errors.DimensionError, instead of possibly needing to import uxarray.errors specifically and then do uxarray.errors.DimensionError. Questions for reviewers: (A) is this okay to bundle here? (B) Do I also need to add something to docs/api.rst page?

Below copies the list of changes enumerated in issue 1622, and marks them as (done), possibly with minor notes to show how they have been completed here.

  1. (done) Exception → ValueError in grid.bounds.insert_pt_in_latlonbox: "Longitude point out of range"
  2. (done - chose AssertionError) Exception → AssertionError or ValueError(?) in grid.bounds.insert_pt_in_latlonbox: "Logic error in longitude box width calculation"
  3. (done) Exception → AttributeError in property Grid.inverse_indices: "Grid is not a subset, therefore no inverse face indices exist"
  4. (done) ValueError → TypeError in core.aggregation._node_to_face_aggregation and core.aggregation._node_to_edge_aggregation: (error currently has no message, but is raised to indicate: not isinstance(uxda.data, (np.ndarray, da.Array)).)
  5. (done) ValueError → TypeError in UxDataArray.zonal_anomaly: "Invalid value for 'lat'. Must be a tuple (start, end, step) or array-like band edges."
  6. (done) ValueError → TypeError in Grid.from_face_vertices: "Input must be either a list, tuple, or np.ndarray"
  7. (done) ValueError → NotImplementedError in Grid.boundary_node_indices: (error currently has no message, but seems like it is supposed to indicate that construction of boundary_node_indices is not yet implemented.)
  8. (done) TypeError → ValueError in grid.neighbors.KDTree._build_from_nodes: f"Unknown coordinate_system, {self.coordinate_system}, use either 'cartesian' or 'spherical'".
  9. (done) TypeError → ValueError in grid.neighbors.KDTree._current_tree and grid.neighbors.BallTree._current_tree: f"Unknown coordinates location, {self._coordinates}, use either 'nodes', 'face centers', or 'edge centers'"
  10. (done) ValueError → NotImplementedError in grid.slice._slice_node_indices, grid.slice._slice_node_indices, and grid.slice._slice_face_indices: "Exclusive slicing is not yet supported."
  11. (done) ValueError → TypeError in grid.utils.make_setter: f"{key} must be an xr.DataArray" (raised if not isinstance(value, xr.DataArray)).
  12. (done) ValueError → NotImplementedError in io._esmf._read_esmf: "Reading in ESMF grids with Cartesian coordinates not yet supported"
  13. (done) ValueError → NotImplementedError in io._icon._read_icon: "Conversion of the ICON Dual mesh is not yet supported."
  14. (done) Exception → NotImplementedError in io._scrip._to_ugrid: "Structured scrip files are not yet supported"
  15. (done) ValueError → AttributeError in remap.yac._get_lon_lat: f"Grid does not provide {lon_attr}/{lat_attr} required for YAC remapping."
  16. (done) RuntimeError → GridInvalidError, in UxDataArray.get_dual, UxDataset.get_dual, and Grid.get_dual: "Duplicate nodes found, cannot construct dual"
  17. (done) RuntimeError → DimensionError, in Grid.from_face_vertices: f"Invalid Input Dimension: {face_vertices.ndim}. Expected dimension should be 3: [n_face, n_node, two/three] or 2 when only one face is passed in."
  18. (done) RuntimeError → GridInvalidError, in Grid.validate: "Mesh validation failed."
  19. (done) AssertionError → DimensionError, in grid.neighbors._prepare_xy_for_query: "The dimension of each coordinate pair must be two (lon, lat). Did you attempt to query using Cartesian (x, y, z) coordinates?", and "The dimension of each coordinate pair must be two (lon, lat).)"
  20. (done) AssertionError → DimensionError, in grid.neighbors._prepare_xyz_for_query: (similar messages as above)
  21. (done) RuntimeError → GridInvalidError, in io.utils._parse_grid_type: "Failed to parse uxgrid information from xarray.Dataset."

PR Checklist

General

  • An issue is created and linked
  • Added appropriate labels (if your uxarray repo permissions allow it)
  • Filled out Overview and Expected Usage (if applicable) sections

Testing & Benchmarking

  • Adequate tests are created if there is new functionality
  • Tests are not too basic (such as simply calling a function and nothing else)
  • [N/A] Tests cover all major paths in your new functions
  • [N/A] If this PR could affect performance, ran ASV benchmarks and confirmed they show expected behavior (add a new benchmark if necessary)

This means users can reliably refer to, e.g., ux.errors.DimensionError, instead of possibly needing to import uxarray.errors specifically and then do uxarray.errors.DimensionError.
@Sevans711 Sevans711 added improvement Improvements on existing features or infrastructure developer experience Makes the codebase easier to read, debug, maintain, or extend. labels Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developer experience Makes the codebase easier to read, debug, maintain, or extend. improvement Improvements on existing features or infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update uxarray error types for cases with misleading/confusing types

1 participant